fix: get sandwich loggic AI -> Algorithm #69
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Walkthrough이 변경은 기존의 샌드위치 휴가 추천 기능에서 외부 MCP 클라이언트 호출 및 관련 데이터 클래스를 제거하고, 내부적으로 Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant RecommendController
participant RecommendServiceImpl
participant SandwichCalculator
Client->>RecommendController: GET /getSandwich
RecommendController->>RecommendServiceImpl: getSandwich()
RecommendServiceImpl->>SandwichCalculator: recommendSandwich(holidays, weekends, ...)
SandwichCalculator-->>RecommendServiceImpl: List<Sandwich>
RecommendServiceImpl-->>RecommendController: SandwichApiResponse
RecommendController-->>Client: ApiResponse<SandwichApiResponse>
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
noweekend-core/core-domain/src/main/kotlin/noweekend/core/domain/sandwich/Sandwich.kt (1)
5-5: 간단하고 명확한 도메인 모델입니다.날짜 범위를 나타내는 간단한 데이터 클래스로 적절하게 구현되었습니다. 필요시 시작일이 종료일보다 앞서야 한다는 비즈니스 규칙을 추가하는 것을 고려해볼 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
noweekend-clients/client-mcp/src/main/kotlin/noweekend/client/mcp/recommend/RecommendApi.kt(0 hunks)noweekend-clients/client-mcp/src/main/kotlin/noweekend/client/mcp/recommend/RecommendClient.kt(0 hunks)noweekend-clients/client-mcp/src/main/kotlin/noweekend/client/mcp/recommend/model/Sandwich.kt(0 hunks)noweekend-core/core-api/src/main/kotlin/noweekend/core/api/config/SecurityConfig.kt(1 hunks)noweekend-core/core-api/src/main/kotlin/noweekend/core/api/controller/TempController.kt(3 hunks)noweekend-core/core-api/src/main/kotlin/noweekend/core/api/controller/v1/RecommendController.kt(2 hunks)noweekend-core/core-api/src/main/kotlin/noweekend/core/api/controller/v1/docs/RecommendControllerDocs.kt(2 hunks)noweekend-core/core-api/src/main/kotlin/noweekend/core/api/controller/v1/response/SandwichResponse.kt(1 hunks)noweekend-core/core-api/src/main/kotlin/noweekend/core/domain/recommend/RecommendService.kt(1 hunks)noweekend-core/core-api/src/main/kotlin/noweekend/core/domain/recommend/RecommendServiceImpl.kt(4 hunks)noweekend-core/core-domain/src/main/kotlin/noweekend/core/domain/sandwich/Sandwich.kt(1 hunks)noweekend-core/core-domain/src/main/kotlin/noweekend/core/domain/sandwich/SandwichCalculator.kt(1 hunks)noweekend-mcp/mcp-host/src/main/kotlin/noweekend/mcphost/controller/request/SandwichRequest.kt(0 hunks)noweekend-mcp/mcp-host/src/main/kotlin/noweekend/mcphost/service/ChatbotService.kt(2 hunks)
💤 Files with no reviewable changes (4)
- noweekend-clients/client-mcp/src/main/kotlin/noweekend/client/mcp/recommend/RecommendApi.kt
- noweekend-clients/client-mcp/src/main/kotlin/noweekend/client/mcp/recommend/model/Sandwich.kt
- noweekend-mcp/mcp-host/src/main/kotlin/noweekend/mcphost/controller/request/SandwichRequest.kt
- noweekend-clients/client-mcp/src/main/kotlin/noweekend/client/mcp/recommend/RecommendClient.kt
🔇 Additional comments (12)
noweekend-core/core-api/src/main/kotlin/noweekend/core/api/config/SecurityConfig.kt (1)
33-33: 개발 환경에서의 샌드위치 엔드포인트 허용 설정이 적절합니다.개발 환경에서만
/sandwich경로를 허용하여 테스트와 개발 워크플로우를 지원하는 설정이 보안 측면에서 적절합니다.noweekend-mcp/mcp-host/src/main/kotlin/noweekend/mcphost/service/ChatbotService.kt (2)
337-337: 로그 메시지가 일관되게 업데이트되었습니다.재시도 횟수 변경에 따른 로그 메시지 업데이트가 적절합니다.
340-340: 예외 메시지가 일관되게 업데이트되었습니다.재시도 횟수 변경에 따른 예외 메시지 업데이트가 적절합니다.
noweekend-core/core-api/src/main/kotlin/noweekend/core/domain/recommend/RecommendService.kt (2)
5-5: 임포트 경로가 올바르게 업데이트되었습니다.외부 클라이언트 모델에서 내부 응답 패키지로 임포트가 변경되어 아키텍처 개선과 일치합니다.
13-13: 구현체 및 컨트롤러에서 getSandwich 시그니처 일관 적용 확인됨
RecommendService인터페이스 변경 사항이RecommendServiceImpl.kt와RecommendController.kt에 모두 반영되어,getSandwich()메서드가 파라미터 없이 일관되게 호출되고 있습니다. 추가 수정은 필요 없어 보입니다.noweekend-core/core-api/src/main/kotlin/noweekend/core/api/controller/v1/docs/RecommendControllerDocs.kt (1)
11-11: 임포트 경로가 올바르게 업데이트되었습니다.외부 클라이언트 모델에서 내부 응답 패키지로 임포트가 변경되어 아키텍처 개선과 일치합니다.
noweekend-core/core-api/src/main/kotlin/noweekend/core/api/controller/v1/response/SandwichResponse.kt (1)
1-14: DTO 구조가 명확하고 적절합니다!휴가 추천 응답을 위한 데이터 클래스가 깔끔하게 구현되었습니다.
noweekend-core/core-api/src/main/kotlin/noweekend/core/api/controller/TempController.kt (1)
49-53: 샌드위치 추천 엔드포인트가 잘 구현되었습니다.
RecommendService를 통해 추천 결과를 반환하는 구조가 적절합니다.이 엔드포인트가 인증 없이 접근 가능한 점을 확인해주세요.
TempController가 임시 컨트롤러라고 해도 보안 관점에서 검토가 필요할 수 있습니다.noweekend-core/core-api/src/main/kotlin/noweekend/core/api/controller/v1/RecommendController.kt (1)
50-54: 사용자 ID 의존성이 성공적으로 제거되었습니다.기존의 하드코딩된 mock 응답이 제거되고 서비스 레이어를 직접 호출하는 구조로 개선되었습니다.
noweekend-core/core-api/src/main/kotlin/noweekend/core/domain/recommend/RecommendServiceImpl.kt (2)
52-52: SandwichCalculator 의존성 주입이 적절합니다.생성자 주입을 통해
calculator가 올바르게 주입되었습니다.
211-248: 샌드위치 휴가 추천 로직이 잘 구현되었습니다.AI 기반 추천에서 도메인 특화 알고리즘으로의 전환이 성공적으로 이루어졌습니다. 특히:
- 공휴일과 주말 데이터를 활용한 계산 로직이 명확합니다
- 연차 사용일 계산이 공휴일과 주말을 제외하고 정확히 수행됩니다
- 전체 휴가 일수 계산에 inclusive 방식(+1)이 올바르게 적용되었습니다
noweekend-core/core-domain/src/main/kotlin/noweekend/core/domain/sandwich/SandwichCalculator.kt (1)
15-108: Sandwich 추천 알고리즘 및 30회 재시도 로직 확인 완료알고리즘 구현이 복잡하지만 논리적으로 타당합니다.
- 공휴일 전후 갭을 고려한 후보 날짜 생성
- 최소 기간 및 최대 갭 제약 검증
- 중복 기간 그룹화 및 랜덤 선택
- PR 설명에 언급된 30회 재시도 메커니즘은
noweekend-mcp/mcp-host/src/main/kotlin/noweekend/mcphost/service/ChatbotService.kt
의repeat(30) { … println("getSandwich retry ${attempt + 1}/30") … }블록에서 이미 구현되어 있습니다.위 사항 확인되었으므로 추가 수정은 필요 없습니다.
|
|
||
| var lastException: Throwable? = null | ||
| repeat(10) { attempt -> | ||
| repeat(30) { attempt -> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
재시도 횟수가 과도하고 백오프 전략이 없습니다.
30번의 재시도는 과도할 수 있으며, 연속적인 실패 시 리소스 낭비가 발생할 수 있습니다. 백오프 전략을 추가하는 것을 권장합니다.
다음과 같이 백오프 전략을 추가하는 것을 고려해보세요:
+val backoff = listOf(1000L, 2000L, 4000L, 8000L, 16000L)
repeat(30) { attempt ->
try {
// ... 기존 로직 ...
} catch (e: Throwable) {
lastException = e
println("getSandwich retry ${attempt + 1}/30: ${e.message}")
+ if (attempt < 29) {
+ Thread.sleep(backoff.getOrElse(attempt) { backoff.last() })
+ }
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| repeat(30) { attempt -> | |
| // simple exponential backoff delays (in milliseconds) | |
| val backoff = listOf(1000L, 2000L, 4000L, 8000L, 16000L) | |
| repeat(30) { attempt -> | |
| try { | |
| // ... 기존 로직 ... | |
| } catch (e: Throwable) { | |
| lastException = e | |
| println("getSandwich retry ${attempt + 1}/30: ${e.message}") | |
| if (attempt < 29) { | |
| // delay before next retry | |
| Thread.sleep(backoff.getOrElse(attempt) { backoff.last() }) | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In
noweekend-mcp/mcp-host/src/main/kotlin/noweekend/mcphost/service/ChatbotService.kt
at line 310, the current retry loop repeats 30 times without any backoff
strategy, which can lead to resource waste on continuous failures. Modify the
retry logic to include an exponential backoff delay between attempts, increasing
the wait time progressively after each failure to reduce resource strain and
improve stability.
| fun getSandwich( | ||
| @Parameter(hidden = true) @CurrentUserId userId: String, | ||
| ): ApiResponse<SandwichApiResponse> | ||
| fun getSandwich(): ApiResponse<SandwichApiResponse> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
메서드 시그니처 변경이 적절하지만 문서화 내용과 불일치합니다.
userId 파라미터가 제거된 것은 내부 계산 로직으로의 변경과 일치하지만, 문서화 내용에서 여전히 "유저의 남은 연차"를 언급하고 있어 실제 구현과 일치하지 않습니다.
문서화 내용을 실제 구현에 맞게 수정하세요:
@Operation(
- summary = "유저의 남은 연차와 올해 남은 공휴일/주말로 샌드위치 휴가(bridge vacation) 추천",
+ summary = "올해 남은 공휴일/주말로 샌드위치 휴가(bridge vacation) 추천",
description = """
-유저의 남은 연차, 올해 남은 공휴일/주말을 바탕으로, 연속으로 쉴 수 있는 휴가(샌드위치 휴가) 구간을 추천합니다.
+올해 남은 공휴일/주말을 바탕으로, 연속으로 쉴 수 있는 휴가(샌드위치 휴가) 구간을 추천합니다.
각 휴가 구간별 실제 사용 연차 일수와 전체 휴가 일수도 반환됩니다.
""",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| fun getSandwich(): ApiResponse<SandwichApiResponse> | |
| @Operation( | |
| - summary = "유저의 남은 연차와 올해 남은 공휴일/주말로 샌드위치 휴가(bridge vacation) 추천", | |
| + summary = "올해 남은 공휴일/주말로 샌드위치 휴가(bridge vacation) 추천", | |
| - description = """ | |
| -유저의 남은 연차, 올해 남은 공휴일/주말을 바탕으로, 연속으로 쉴 수 있는 휴가(샌드위치 휴가) 구간을 추천합니다. | |
| + description = """ | |
| +올해 남은 공휴일/주말을 바탕으로, 연속으로 쉴 수 있는 휴가(샌드위치 휴가) 구간을 추천합니다. | |
| +각 휴가 구간별 실제 사용 연차 일수와 전체 휴가 일수도 반환됩니다. | |
| """, | |
| ) | |
| fun getSandwich(): ApiResponse<SandwichApiResponse> |
🤖 Prompt for AI Agents
In
noweekend-core/core-api/src/main/kotlin/noweekend/core/api/controller/v1/docs/RecommendControllerDocs.kt
at line 323, the method signature for getSandwich no longer includes the userId
parameter, but the documentation still references "user's remaining annual
leave." Update the documentation to remove any mention of userId or
user-specific data and align the description with the current implementation
that does not require user identification.
Comment on lines
+129
to
+181
| private fun extendPeriod( | ||
| startDate: LocalDate, | ||
| holidays: Set<LocalDate>, | ||
| weekends: Set<LocalDate>, | ||
| globalMaxGap: Int, | ||
| until: LocalDate, | ||
| specials: List<LocalDate>, | ||
| ): Sandwich { | ||
| val isPreGapStart = startDate !in holidays && startDate !in weekends | ||
|
|
||
| var gapCount = 0 | ||
| var contigCount = 0 | ||
| var lastWasGap = false | ||
|
|
||
| var current = startDate | ||
| var endDate = startDate | ||
|
|
||
| while (!current.isAfter(until)) { | ||
| val isHoliday = current in holidays | ||
| val isWeekend = current in weekends | ||
|
|
||
| if (isHoliday || isWeekend) { | ||
| // reset gap, increment contiguous holiday/weekend count | ||
| contigCount = if (lastWasGap) 1 else contigCount + 1 | ||
| gapCount = 0 | ||
| lastWasGap = false | ||
| endDate = current | ||
| } else { | ||
| if (!lastWasGap) gapCount = 0 | ||
| gapCount++ | ||
|
|
||
| val currentGapMax = when { | ||
| contigCount >= 3 && isPreGapStart -> 0 | ||
| contigCount >= 3 -> 1 | ||
| else -> globalMaxGap | ||
| } | ||
| if (gapCount > currentGapMax) break | ||
|
|
||
| if (contigCount < 3) { | ||
| val nextSpecial = specials.firstOrNull { it > current } | ||
| val window = (currentGapMax - gapCount + 1).toLong() | ||
| if (nextSpecial == null || nextSpecial.isAfter(current.plusDays(window))) { | ||
| break | ||
| } | ||
| } | ||
|
|
||
| lastWasGap = true | ||
| endDate = current | ||
| } | ||
| current = current.plusDays(1) | ||
| } | ||
| return Sandwich(startDate, endDate) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
extendPeriod 메서드의 복잡한 로직에 주석 추가를 권장합니다.
이 메서드는 휴가 기간을 확장하는 핵심 로직을 담고 있지만, 특히 lines 160-164의 조건부 갭 최대값 설정 부분이 이해하기 어렵습니다.
val currentGapMax = when {
+ // 3일 이상 연속 휴일 블록 + pre-gap 시작인 경우: 추가 갭 허용 안 함
contigCount >= 3 && isPreGapStart -> 0
+ // 3일 이상 연속 휴일 블록인 경우: 1일 갭만 허용
contigCount >= 3 -> 1
+ // 그 외의 경우: 전역 최대 갭 허용
else -> globalMaxGap
}🤖 Prompt for AI Agents
In
noweekend-core/core-domain/src/main/kotlin/noweekend/core/domain/sandwich/SandwichCalculator.kt
between lines 129 and 181, the extendPeriod method contains complex logic,
especially around lines 160-164 where the conditional setting of currentGapMax
occurs. Add clear, concise comments explaining the purpose of each condition in
this section and the overall intent of the gap counting and contiguous count
logic to improve code readability and maintainability.
Comment on lines
+76
to
+105
| holidays.forEach { holiday -> | ||
| print(holiday) | ||
| print(" ") | ||
| } | ||
| logger.info("") | ||
| logger.info("=========================== HOLIDAY ===========================") | ||
|
|
||
| logger.info("=========================== WEEKEND ===========================") | ||
| weekends.forEach { weekend -> | ||
| print(weekend) | ||
| print(" ") | ||
| } | ||
| logger.info("") | ||
| logger.info("=========================== WEEKEND ===========================") | ||
| logger.info("") | ||
| logger.info("=========================== rawPeriods ===========================") | ||
| rawPeriods.forEach { | ||
| print(it.startDate) | ||
| print(" ~ ") | ||
| logger.info(it.endDate.toString()) | ||
| } | ||
| logger.info("=========================== rawPeriods ===========================") | ||
|
|
||
| logger.info("=========================== response ===========================") | ||
| response.forEach { | ||
| print(it.startDate) | ||
| print(" ~ ") | ||
| logger.info(it.endDate.toString()) | ||
| } | ||
| logger.info("=========================== response ===========================") |
There was a problem hiding this comment.
로깅 구현을 개선해야 합니다.
print() 문을 사용하는 대신 일관되게 logger를 사용해야 합니다. 현재 구현은 로그 출력이 분산되어 관리가 어렵습니다.
- logger.info("=========================== HOLIDAY ===========================")
- holidays.forEach { holiday ->
- print(holiday)
- print(" ")
- }
- logger.info("")
- logger.info("=========================== HOLIDAY ===========================")
+ logger.info("=========================== HOLIDAY ===========================")
+ logger.info("Holidays: ${holidays.joinToString(", ")}")
+ logger.info("=========================== HOLIDAY ===========================")
- logger.info("=========================== WEEKEND ===========================")
- weekends.forEach { weekend ->
- print(weekend)
- print(" ")
- }
- logger.info("")
- logger.info("=========================== WEEKEND ===========================")
+ logger.info("=========================== WEEKEND ===========================")
+ logger.info("Weekends: ${weekends.joinToString(", ")}")
+ logger.info("=========================== WEEKEND ===========================")
- logger.info("")
- logger.info("=========================== rawPeriods ===========================")
- rawPeriods.forEach {
- print(it.startDate)
- print(" ~ ")
- logger.info(it.endDate.toString())
- }
- logger.info("=========================== rawPeriods ===========================")
+ logger.info("=========================== rawPeriods ===========================")
+ rawPeriods.forEach {
+ logger.info("${it.startDate} ~ ${it.endDate}")
+ }
+ logger.info("=========================== rawPeriods ===========================")
- logger.info("=========================== response ===========================")
- response.forEach {
- print(it.startDate)
- print(" ~ ")
- logger.info(it.endDate.toString())
- }
- logger.info("=========================== response ===========================")
+ logger.info("=========================== response ===========================")
+ response.forEach {
+ logger.info("${it.startDate} ~ ${it.endDate}")
+ }
+ logger.info("=========================== response ===========================")Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
noweekend-core/core-domain/src/main/kotlin/noweekend/core/domain/sandwich/SandwichCalculator.kt
between lines 76 and 105, replace all print() statements with appropriate
logger.info() calls to ensure consistent and centralized logging. This means
converting each print(holiday) and print(" ") to logger.info() with properly
formatted messages, and similarly for weekends, rawPeriods, and response
iterations, so all output is managed through the logger instead of standard
output.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
요약
작업 내용
SandwichCalculator구현 추가getSandwich시그니처 및 DTO 매핑 변경println→logger, retry 30회, 보안 설정 조정리뷰 포인트
Summary by CodeRabbit
신규 기능
기능 개선
버그 수정
기타